Check a pgtest server address before the tests use it by SirLouen · Pull Request #71 · gopherium/framework

SirLouen · 2026-10-12T15:00:27Z

Closes #70
Closes #67

What

pgtest.CheckAddress returns the error New would stop a test with. It covers every address New refuses, postgres.CheckAddress included. Each error matches dbkit.ErrAddress and never repeats the address.
pgtest now refuses a pool key such as pool_max_conns in the server address or its service file. pgtestdb would send it to the server.
NewSwept checks the address before it sweeps, so a refused address never connects.
PostgreSQL 18 reports a RESTRICT stop as 23001. Classify now classes it as dbkit.ErrForeignKey.

Why

Downstream applications read their test server address from a setting of their own. A wrong address should stop the tests with a message that names that setting. A pool_ key used to fail later with a server error. NewSwept also connected and swept before it refused an address. dbkit/sqlite already classes a RESTRICT stop as a foreign key failure.

Testing Instructions

Run the testing toolchain in dbkit/postgres against a PostgreSQL 18 server.

Summary by CodeRabbit

New Features
- Added PostgreSQL test-address validation, with consistent errors for addresses that cannot be used by the test helpers.
- Test helpers now reject pool settings that could move test databases and validate addresses before sweeping.
Bug Fixes
- PostgreSQL RESTRICT constraint violations are now classified as foreign-key errors.

…f pgtest.New

…ost and query checks

…sweep

…ool_ check

…g in the changelog

coderabbitai · 2026-10-12T15:00:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 283cd5ed-b03c-4738-a42d-16b7c6986d6f

📥 Commits

Reviewing files that changed from the base of the PR and between 5424d5d and fcc9698.

📒 Files selected for processing (8)

dbkit/postgres/CHANGELOG.md
dbkit/postgres/classify.go
dbkit/postgres/classify_test.go
dbkit/postgres/pgtest/address.go
dbkit/postgres/pgtest/address_test.go
dbkit/postgres/pgtest/new.go
dbkit/postgres/pgtest/swept.go
dbkit/postgres/pgtest/swept_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The changes add pgtest address validation, run that validation before stale-database sweeping, and map PostgreSQL SQLSTATE 23001 to dbkit.ErrForeignKey.

Changes

pgtest address validation

Layer / File(s)	Summary
Address validation and pool-key checks `dbkit/postgres/pgtest/address.go`, `dbkit/postgres/pgtest/new.go`, `dbkit/postgres/pgtest/address_test.go`, `dbkit/postgres/CHANGELOG.md`	`pgtest.CheckAddress` returns the error from `configOf`. Address validation rejects configured `pool_` keys without a dot, and tests cover refused addresses, message secrecy, and accepted URL forms.
Address check before stale-database sweep `dbkit/postgres/pgtest/swept.go`, `dbkit/postgres/pgtest/swept_test.go`	`NewSwept` checks the age and address before sweeping. Tests cover address refusals and stale-database sweep behavior.

PostgreSQL foreign-key error classification

Layer / File(s)	Summary
Classify RESTRICT violations `dbkit/postgres/classify.go`, `dbkit/postgres/classify_test.go`	SQLSTATE 23001 maps to `dbkit.ErrForeignKey`. Tests cover restricted deletes and updates and check classification behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant newSwept
  participant CheckAddress
  participant configOf
  participant Sweep
  participant New
  Caller->>newSwept: provide address and age
  newSwept->>CheckAddress: validate address
  CheckAddress->>configOf: resolve address configuration
  configOf-->>CheckAddress: return validation result
  CheckAddress-->>newSwept: return validation result
  newSwept->>Sweep: sweep stale databases after successful validation
  newSwept->>New: create test database after sweeping

Merge Risk: ⚪ Minimal · up to fcc96

No actionable merge-blocking issue is established. The PostgreSQL 18 test run remains to be completed as part of normal validation.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 7 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: validating a pgtest server address before tests use it.
Linked Issues check		The changes satisfy issue `#70`. `pgtest.CheckAddress` is exported and checks the refusals made by `New`, including `postgres.CheckAddress` refusals. The new tests verify `dbkit.ErrAddress` matching and…
Out of Scope Changes check		The changes stay within issues `#70` and `#67`. The address parsing refactor supports complete refusal coverage. The `NewSwept` test changes verify validation-before-sweep behavior. The changelog document…

Full details: Docstring Coverage

Explanation

Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 7 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codescene-access

Code Health Improved (3 files improve in Code Health)

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

View Improvements

File	Code Health Impact	Categories Improved
new.go	9.39 → 10.00	Complex Method, Complex Conditional
swept_test.go	9.39 → 10.00	Complex Conditional
classify_test.go	9.69 → 10.00	Complex Method

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

greptile-apps · 2026-10-12T15:12:37Z

Confidence Score: 5/5

[Medium impact] No blocking issues found in the changed address-validation and error-classification behavior. The reviewed changes are safe to merge.

T-Rex evidence

Evidence from the check

The executed shell script runs the requested helper and refusal tests with a nonconnecting placeholder address, making the test invocation reproducible.

Evidence from the check

The executed capture utility records each command, working directory, exit code, and observed output, preserving runtime evidence.

Command output from the check

Both requested test groups ran with verbose output and exited zero, showing that the existing refusal-contract assertions passed.

Evidence from the check

The authored probe supplies a pool key and synthetic password through a service file and records helper output plus TCP connection counts, testing refusal before network activity.

Evidence from the check

The executed runner extracts each revision into a disposable checkout and runs the probe without modifying production files, enabling comparable base and head captures.

Evidence from the check

This is the generated Go test executed against the pre-change revision, preserving the source used for the baseline observation.

Command output from the check

The baseline probe observed connection attempts, synthetic address disclosure, and a sweep-failure log, demonstrating the earlier behavior.

Evidence from the check

This is the generated Go test executed against head with assertions for identical messages, no connections, and no synthetic-secret disclosure, preserving the executed checks.

Command output from the check

The head probe observed identical refusal messages from all three helpers and zero constructor connections with no synthetic-secret disclosure, confirming the scoped contract.

Command output from the check

The command captured the head revision, numbered production and test source, and an empty production diff, tying runtime results to unchanged reviewed files.

Command output from the check

Pulled and started postgres:18 successfully, enabling actual server execution rather than synthetic-only validation.

Evidence from the check

The authored harness generates real RESTRICT errors and synthetic control errors and asserts class, identity, message, and idempotence preservation.

Evidence from the check

The executed script extracts both classifier versions and runs the same race-enabled harness against each, isolating the mapping change.

Evidence from the check

The executed helper saves actual combined command output with command, working directory, and exit code headers, making the logs reproducible.

Evidence from the check

Exact classifier source extracted from the mapping commit's parent was executed in the baseline run, showing the prior behavior.

Command output from the check

Executed real PostgreSQL 18.6 and synthetic tests against the old classifier; 23001 was unclassified while identity and message checks passed.

Evidence from the check

Exact HEAD classifier source was executed in the after run and matches the checkout byte-for-byte, validating the changed implementation.

Command output from the check

Executed the same real and synthetic tests with the new mapping; 23001 matched ErrForeignKey and all preservation and control assertions passed.

Command output from the check

Ran only the checkout's restricted delete and update classification tests against the disposable server with the race detector, and both passed.

Command output from the check

Checked untouched production files, compared executed sources and hashes, and removed the container and volumes, confirming an isolated one-line validation.

Summary

Adds pgtest.CheckAddress so callers can check their test server setting before creating a database.

Callers can check a test database address before using it.
NewSwept checks the address before it sweeps stale databases.
RESTRICT errors now count as foreign-key errors.

Focused address tests passed. A service-file check confirmed that refused pool settings produce the same secret-safe message from all three helpers, with no connection attempts from either constructor. PostgreSQL 18.6 RESTRICT delete and update tests passed, including checks that classification preserves the original error, message, and repeated-classification behavior.

No blocking issues found. The reviewed changes are safe to merge.

_{Reviews (1) · Last reviewed commit: "docs(dbkit/postgres): log the 23001 clas..." · Reviewed by Greptile}

SirLouen added 9 commits October 12, 2026 15:30

feat(dbkit/postgres): export pgtest.CheckAddress with every refusal o…

47d2c77

…f pgtest.New

refactor(dbkit/postgres): split pgtest's serverURL into its scheme, h…

216da5a

…ost and query checks

feat(dbkit/postgres): refuse a pool_ key in a pgtest server address

a105aee

test(dbkit/postgres): split the compound checks of the NewSwept tests

db9f342

fix(dbkit/postgres): check the address in pgtest.NewSwept before the …

dde4501

…sweep

test(dbkit/postgres): move the TestClassify case checks into a helper

f888bad

fix(dbkit/postgres): let a dotted server setting through the pgtest p…

f5534d5

…ool_ check

fix(dbkit/postgres): class the RESTRICT stop 23001 as ErrForeignKey

23d3d09

docs(dbkit/postgres): log the 23001 class and the dotted pool_ settin…

fcc9698

…g in the changelog

SirLouen self-assigned this Oct 12, 2026

codescene-access Bot approved these changes Oct 12, 2026

View reviewed changes

SirLouen merged commit f20b500 into main Oct 12, 2026
38 checks passed

SirLouen deleted the feat/70-pgtest-address branch October 12, 2026 15:35

Navigation Menu

Repository navigation

Check a pgtest server address before the tests use it - #71

Check a pgtest server address before the tests use it#71
SirLouen merged 9 commits into
mainfrom
feat/70-pgtest-address

SirLouen commented Oct 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codescene-access Bot left a comment

Uh oh!

greptile-apps Bot commented Oct 12, 2026 •

edited

Loading

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

SirLouen commented Oct 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Testing Instructions

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codescene-access Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

T-Rex evidence

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

SirLouen commented Oct 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 12, 2026 •

edited

Loading

greptile-apps Bot commented Oct 12, 2026 •

edited

Loading